Hurricup/fixes #2952
Hurricup/fixes #2952
Conversation
WalkthroughThis pull request encompasses modifications across multiple files in the Perl plugin, focusing on workflow configuration, lexer updates, module extension refinement, project generator peer modifications, and test resource additions. The changes primarily involve enhancing syntax handling, updating component initialization logic, and expanding test coverage for Perl language parsing and regex interpolation scenarios. Changes
Possibly related issues
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
plugin/src/test/resources/unit/perl/parser/indexesAsRegexInterpolated.code (1)
10-17: Consider adding edge cases for more thorough testing.While the current test cases are good, consider adding:
- Tests with nested array/hash access
- Tests with complex expressions in brackets
- Tests with special regex characters in the keys
Example additions:
$test[/$other[1]/]; $hash{/foo\/bar/}; $array[$x + 1];plugin/core/src/main/java/com/perl5/lang/perl/idea/configuration/module/PerlProjectGeneratorPeerBase.java (1)
43-47: Consider adding @OverRide annotation.The method overloads the parent's method but missing the @OverRide annotation.
+ @Override public @NotNull JComponent getComponent(@NotNull TextFieldWithBrowseButton myLocationField, @NotNull Runnable checkValid) {plugin/src/test/resources/highlighting/perl/syntax/indexesAsRegexInterpolated.pl.txt (1)
45-64: Consider adding edge cases for regex patterns.While the basic regex interpolation is covered, consider adding test cases for:
- Complex regex patterns with metacharacters
- Nested brackets in regex patterns
plugin/core/grammar/Perl.flex (2)
148-148: LGTM! Consider adding documentationThe addition of '/' to REGEX_ARRAY_NEGATING is correct for handling array interpolation in regex patterns. Consider adding a comment explaining the role of each character in the pattern for future maintenance.
Add a comment like:
+ // Characters that negate array interpolation in regex context: + // ^ (caret), : (colon), \ (backslash), [ (left bracket), { (left brace), + // . (dot), , (comma), ' (single quote), " (double quote), } (right brace), + // ) (right paren), / (forward slash) REGEX_ARRAY_NEGATING = [\^\:\\\[\{\.\,\'\"})/]
149-149: LGTM! Consider adding documentationThe addition of '/' to REGEX_HASH_NEGATING is correct for handling hash interpolation in regex patterns. Consider adding a comment explaining the role of each character in the pattern for future maintenance.
Add a comment like:
+ // Characters that negate hash interpolation in regex context: + // ^ (caret), : (colon), \ (backslash), [ (left bracket), { (left brace), + // . (dot), , (comma), / (forward slash) REGEX_HASH_NEGATING = [\^\:\\\[\{\.\,/]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
plugin/core/src/main/gen/com/perl5/lang/perl/lexer/PerlLexer.javais excluded by!**/gen/**
📒 Files selected for processing (9)
.github/workflows/_coverage.yml(1 hunks)plugin/core/grammar/Perl.flex(1 hunks)plugin/core/src/main/java/com/intellij/openapi/projectRoots/impl/PerlModuleExtension.java(0 hunks)plugin/core/src/main/java/com/perl5/lang/perl/idea/configuration/module/PerlProjectGeneratorPeer.java(2 hunks)plugin/core/src/main/java/com/perl5/lang/perl/idea/configuration/module/PerlProjectGeneratorPeerBase.java(4 hunks)plugin/src/test/java/unit/perl/parser/PerlParserLikeTest.java(1 hunks)plugin/src/test/resources/highlighting/perl/syntax/indexesAsRegexInterpolated.pl.txt(1 hunks)plugin/src/test/resources/unit/perl/parser/indexesAsRegexInterpolated.code(1 hunks)plugin/src/test/resources/unit/perl/parser/indexesAsRegexInterpolated.txt(1 hunks)
💤 Files with no reviewable changes (1)
- plugin/core/src/main/java/com/intellij/openapi/projectRoots/impl/PerlModuleExtension.java
🔇 Additional comments (13)
plugin/src/test/resources/unit/perl/parser/indexesAsRegexInterpolated.code (1)
1-8: LGTM! Comprehensive test coverage for array access in regex contexts.
The test cases effectively cover array access within different regex contexts including direct access, string interpolation, and various Perl operators.
plugin/core/src/main/java/com/perl5/lang/perl/idea/configuration/module/PerlProjectGeneratorPeer.java (1)
19-25: LGTM! Clean implementation of UI layout changes.
The addition of VerticalFlowLayout is appropriate for form-based UI components. The constructor change properly integrates with the superclass modifications.
Let's verify other potential layout usages:
✅ Verification successful
VerticalFlowLayout usage is consistent across the codebase
The verification shows that VerticalFlowLayout is consistently used across multiple UI components in the codebase, including configuration panels, settings editors, and debugger interfaces. The implementation in PerlProjectGeneratorPeer follows the same pattern as other similar components, particularly in configuration-related classes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other VerticalFlowLayout usages to ensure consistency
rg "VerticalFlowLayout" --type java
Length of output: 3050
.github/workflows/_coverage.yml (1)
63-63: LGTM! Enhanced security check for SonarCube analysis.
The additional token validation ensures that the SonarCube analysis only runs when all required credentials are present, preventing incomplete or failed analysis attempts.
plugin/core/src/main/java/com/perl5/lang/perl/idea/configuration/module/PerlProjectGeneratorPeerBase.java (4)
31-34: LGTM: Field declaration and initialization look good.
The new field myMainComponent is properly declared as final and annotated with @NotNull.
36-40: LGTM: Constructor implementation is correct.
The constructor properly:
- Accepts and validates the mainComponent parameter
- Calls super constructor
- Initializes fields
67-69: LGTM: Component creation is simplified.
The method now directly returns myMainComponent, which is cleaner than the previous implementation.
60-63: Verify the component initialization order.
The method now adds components to myMainComponent directly. Ensure this doesn't affect the initialization order of UI components.
✅ Verification successful
Based on the search results, I can now provide a final response:
The component initialization order is correctly maintained
The code follows a proper initialization pattern where:
myMainComponentis initialized in the constructorinitializeComponent()is lazily called viamyComponentProvider- The component hierarchy is maintained with
mySdkConfigurablebeing added as a child component - The final component is exposed through
createComponent()which returns the fully initializedmyMainComponent
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of initializeComponent to verify initialization order
ast-grep --pattern 'class $_ {
$$$
initializeComponent($$$) {
$$$
}
$$$
}'
Length of output: 88
Script:
#!/bin/bash
# Let's try a different approach to find usages of initializeComponent and myMainComponent
rg -A 5 "initializeComponent|myMainComponent" --type java
# Also search for the base class or interface that might define this method
rg -B 2 "class PerlProjectGeneratorPeerBase" --type java
Length of output: 4540
plugin/src/test/resources/highlighting/perl/syntax/indexesAsRegexInterpolated.pl.txt (1)
1-16: LGTM: Array element regex interpolation test cases are comprehensive.
The test cases properly cover:
- Basic array element access with regex
- Proper token highlighting for regex delimiters
- Proper token highlighting for array syntax
plugin/src/test/resources/unit/perl/parser/indexesAsRegexInterpolated.txt (3)
1-100: LGTM: AST structure for array element regex interpolation is well-defined.
The parser test correctly defines:
- PsiPerlArrayElementImpl structure
- PsiPerlMatchRegexImpl structure
- Proper nesting of elements
101-199: LGTM: AST structure for hash element regex interpolation is comprehensive.
The test cases properly cover:
- Hash element access with regex
- String interpolation within hash context
- Various regex operations in hash context
200-203: LGTM: POD documentation is properly structured.
The POD section correctly documents all test cases with their expected syntax.
plugin/src/test/java/unit/perl/parser/PerlParserLikeTest.java (1)
29-30: Verify test resources for regex interpolation test
The test method follows the class's pattern, but ensure the corresponding test resources exist:
plugin/src/test/resources/unit/perl/parser/indexesAsRegexInterpolated.codeplugin/src/test/resources/unit/perl/parser/indexesAsRegexInterpolated.txt
✅ Verification successful
Test resources are properly set up
The required test resource files are present in the expected location:
plugin/src/test/resources/unit/perl/parser/indexesAsRegexInterpolated.codeplugin/src/test/resources/unit/perl/parser/indexesAsRegexInterpolated.txt
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the required test resources exist
# Check for test resource files
fd -t f "indexesAsRegexInterpolated" plugin/src/test/resources/unit/perl/parser/
Length of output: 229
plugin/core/grammar/Perl.flex (1)
148-149: Verify fix addresses referenced issues
The changes to both regex negation patterns look correct and are well-tested. Please ensure these changes address the issues mentioned in the PR description (#2946, #2948, #2915).
✅ Verification successful
Let me analyze the issue context and verify the changes. Looking at the issues:
- Issue The parser does not understand the delimiter curly bracket in a regular expression. #2948 is directly relevant to our changes as it describes a bug where the parser doesn't correctly handle delimiter curly brackets in regular expressions, specifically with a slash character inside character classes.
Let me verify if there are any related test cases for this fix.
Let me check for test files specifically related to the lexer and regex parsing.
Let me check the parser test files for regex-related test cases.
Based on the analysis of the code and test files, I can now provide a final response:
Changes correctly address regex delimiter handling
The modifications to REGEX_ARRAY_NEGATING and REGEX_HASH_NEGATING patterns are verified to be correct. The changes:
- Fix the handling of curly bracket delimiters in regular expressions (issue The parser does not understand the delimiter curly bracket in a regular expression. #2948)
- Are supported by comprehensive test coverage in
charGroups.codeandsuperExtendedRegexp.code - Properly handle character classes with negation and special characters
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the referenced issues for context
# Fetch issue details
gh issue view 2946
gh issue view 2948
gh issue view 2915
Length of output: 14328
Script:
#!/bin/bash
# Check for related test files and test cases
rg -l "s\{.*\[.*\].*\}" --type perl
rg -l "regex.*negat" --type perl
Length of output: 988
Script:
#!/bin/bash
# Look for test files related to regex parsing
fd -e "t" -e "test" | rg -i "regex|lexer"
# Check for recent changes in test files
fd "test" | xargs rg -l "\[\^.*\]"
Length of output: 15360
Script:
#!/bin/bash
# Check parser test files for regex-related tests
cat plugin/src/test/resources/unit/perl/parser/charGroups.code plugin/src/test/resources/unit/perl/parser/charGroups.txt
# Check for recent changes in regex-related test files
cat plugin/src/test/resources/unit/perl/parser/superExtendedRegexp.code plugin/src/test/resources/unit/perl/parser/superExtendedRegexp.txt
Length of output: 16829
Qodana for JVM1437 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Detected 11 dependenciesThird-party software listThis page lists the third-party software dependencies used in perl5
Contact Qodana teamContact us at [email protected]
|
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores